Skip to content

[Fix #1569] OneOf builder null value in serialization#1570

Merged
fjtirado merged 2 commits into
open-workflow-specification:mainfrom
ricardozanini:fix/1569-oneof-builder-null-value
Jul 24, 2026
Merged

[Fix #1569] OneOf builder null value in serialization#1570
fjtirado merged 2 commits into
open-workflow-specification:mainfrom
ricardozanini:fix/1569-oneof-builder-null-value

Conversation

@ricardozanini

Copy link
Copy Markdown
Collaborator

Summary

  • ReferenceableAuthenticationPolicyBuilder.build() and BackoffBuilder.build() call all OneOf setters unconditionally, causing the last setter (with null/empty) to overwrite OneOfValueProvider.value — leading to null or wrong values during serialization.
  • Fix: add null guards in build() so only the configured setter is called. Lazy-init backoff objects in BackoffBuilder to avoid empty defaults.
  • Add tests verifying OneOfValueProvider.get() returns the correct value.

Fixes #1569

Copilot AI review requested due to automatic review settings July 23, 2026 16:06
@ricardozanini
ricardozanini requested a review from fjtirado as a code owner July 23, 2026 16:06
…rialization

Fix ReferenceableAuthenticationPolicyBuilder and BackoffBuilder
calling all OneOf setters unconditionally, which clobbers the
value field used by serialization.

- Add null guards in ReferenceableAuthenticationPolicyBuilder.build()
- Lazy-init backoff objects and add null guards in BackoffBuilder
- Add tests verifying OneOfValueProvider.get() returns correct value

Signed-off-by: Ricardo Zanini <[email protected]>
@ricardozanini
ricardozanini force-pushed the fix/1569-oneof-builder-null-value branch from 8ca8c80 to e134543 Compare July 23, 2026 16:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a serialization bug where fluent builders for oneOf model types were calling multiple setters unconditionally, causing later null/empty setters to overwrite the internally selected oneOf value and resulting in incorrect OneOfValueProvider.get() output.

Changes:

  • Add null-guards in ReferenceableAuthenticationPolicyBuilder.build() so only the configured oneOf setter is invoked.
  • Lazy-initialize retry backoff variants and only set the configured variant in BackoffBuilder.build() to avoid empty defaults clobbering the oneOf selection.
  • Add tests asserting OneOfValueProvider.get() is non-null and that only the intended oneOf branch is populated for authentication and retry backoff.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
fluent/spec/src/main/java/io/serverlessworkflow/fluent/spec/ReferenceableAuthenticationPolicyBuilder.java Avoids clobbering oneOf value by only setting the non-null authentication option.
fluent/spec/src/main/java/io/serverlessworkflow/fluent/spec/BaseTryTaskBuilder.java Lazy-inits backoff variants and avoids overwriting the selected backoff oneOf value during build.
fluent/spec/src/test/java/io/serverlessworkflow/fluent/spec/dsl/CallHttpAuthDslTest.java Adds regression coverage ensuring authentication oneOf value is set for serialization.
fluent/spec/src/test/java/io/serverlessworkflow/fluent/spec/dsl/TryCatchDslTest.java Adds regression coverage ensuring retry backoff oneOf value is set for serialization.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot AI review requested due to automatic review settings July 23, 2026 16:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

fluent/spec/src/main/java/io/serverlessworkflow/fluent/spec/BaseTryTaskBuilder.java:384

  • BackoffBuilder.build() currently selects the first non-null backoff variant (constant > exponential > linear). If a caller configures more than one variant in the same backoff(b -> ...) consumer (which the public API allows), the later configuration can be silently ignored (e.g., calling exponential(...) after constant(...) will still build constantBackoff). Since RetryBackoff is a oneOf, it would be safer to either (a) fail fast when multiple variants are configured, or (b) ensure the last configured variant wins by clearing the other variants when setting one.
    public RetryBackoff build() {
      if (this.constantBackoff != null) {
        this.retryBackoff.setConstantBackoff(constantBackoff);
      } else if (this.exponentialBackOff != null) {
        this.retryBackoff.setExponentialBackOff(exponentialBackOff);

BackoffBuilder now throws IllegalStateException when mixing backoff
variants (constant, exponential, linear), enforcing the oneOf contract
at build time instead of silently picking the first non-null.

Signed-off-by: Ricardo Zanini <[email protected]>
Copilot AI review requested due to automatic review settings July 23, 2026 19:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

@fjtirado
fjtirado merged commit fe342ee into open-workflow-specification:main Jul 24, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OneOfValueProvider builders clobber value by calling all setters unconditionally

3 participants